Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

3350 tool to automatically convert obsolete variables from input file #3397

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

ym1906
Copy link
Collaborator

@ym1906 ym1906 commented Nov 14, 2024

I modified this validate_input function and added a CLI argument "replace_obsolete"

Now if you pass this argument when you run PROCESS it will automatically convert the variables, spit out the conversion in the terminal and write a comment in the input file where the name was changed.

@ym1906 ym1906 linked an issue Nov 14, 2024 that may be closed by this pull request
@codecov-commenter
Copy link

codecov-commenter commented Nov 14, 2024

Codecov Report

Attention: Patch coverage is 9.52381% with 38 lines in your changes missing coverage. Please review.

Project coverage is 28.43%. Comparing base (d9801ca) to head (b979161).
Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
process/main.py 9.52% 38 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3397      +/-   ##
==========================================
+ Coverage   27.71%   28.43%   +0.71%     
==========================================
  Files          77       77              
  Lines       18084    19215    +1131     
==========================================
+ Hits         5012     5463     +451     
- Misses      13072    13752     +680     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

Copy link
Contributor

@timothy-nunn timothy-nunn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have found several bugs that its not unreasonable to expect will occur in real-world input files.

I need to additionally consider the merits of doing this in-situ while running PROCESS versus this being a script that produces another input file.

process/main.py Outdated Show resolved Hide resolved
process/main.py Outdated Show resolved Hide resolved
changes_made.append(
f"Commented out obsolete variable: {variable_name}"
)
else:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the replacement is a list?

Having, for example, the variable thshield in the input file causes a TypeError: replace() argument 2 must be str, not list

Copy link
Collaborator Author

@ym1906 ym1906 Nov 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hadn't considered that there were lists of replacements. Not sure there is an ideal solution there... I don't really like the idea of writing the new variables with "wrong" values (for ex: the original value of replaced variable). I could just write them without values but this will "break" input file. Maybe this is acceptable to force the user to intervene.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added error message for when there is a list of replacements.

process/main.py Outdated
continue

# Extract the variable name before the separator
variable_name = line.strip().split(" ", 1)[0]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Putting fdeut=0.6 or fdeut= 0.6 in the input file causes

Unknown variable in input file: fdeut                                         
 Error occurred at this line in the IN.DAT file:         449
 fdeut=0.6                                      

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I improved this to detect these situations.

…ment. Modified parsing to find variables with spacing around equals. Added error for when there is a list of replacements
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tool to automatically convert obsolete variables from input file
3 participants